-
Notifications
You must be signed in to change notification settings - Fork 111
rust: drm: gpuvm: Add a missing lock #433
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: asahi-wip
Are you sure you want to change the base?
rust: drm: gpuvm: Add a missing lock #433
Conversation
Shutting these down breaks dwc3 init done by the firmware. We probably never want to do this anyway. It might be possible remove this once a PHY driver is in place to do the init properly, but it may not be worth it. Signed-off-by: Hector Martin <[email protected]>
Signed-off-by: Janne Grunau <[email protected]>
Signed-off-by: Janne Grunau <[email protected]>
Signed-off-by: Janne Grunau <[email protected]>
RTKit is Apple's proprietary real-time operating system framework, used across many subdevices on Apple Silicon platforms including NVMe, system management, GPU, etc. Add Rust abstractions for this subsystem, so that it can be used by upcoming Rust drivers. Note: Although ARM64 support is not yet merged, this can be built on amd64 with CONFIG_COMPILE_TEST=y. FIXME: order in drivers/soc/apple/Kconfig to avoid merge conflicts in asahi tree Signed-off-by: Asahi Lina <[email protected]>
Switch to iosys_map Depends on "rust: Introduce iosys_map bindings" from "[PATCH v6 0/8] Rust bindings for gem shmem + iosys_map" Signed-off-by: Janne Grunau <[email protected]>
This abstraction enables Rust drivers to walk Device Tree nodes and query their properties. Signed-off-by: Asahi Lina <[email protected]>
Some C functions like of_reserved_mem_region_to_resource_byname() expect a pointer to a `struct resource` so provide ::zeroed() as initialiser and ::as_raw() so other parts in the kernel crate can use functions which expect such a pointer. Signed-off-by: Janne Grunau <[email protected]>
Creates Resource from a reserved memory region. Depends on commit f4fcfdd ("of: reserved_mem: Add functions to parse "memory-region"") from v6.16-rc1. Signed-off-by: Janne Grunau <[email protected]>
Use FwNode based device properties instead. Signed-off-by: Janne Grunau <[email protected]>
By analogy to AlwaysRefCounted and ARef, an Ownable type is a (typically C FFI) type that *may* be owned by Rust, but need not be. Unlike AlwaysRefCounted, this mechanism expects the reference to be unique within Rust, and does not allow cloning. Conceptually, this is similar to a KBox<T>, except that it delegates resource management to the T instead of using a generic allocator. Signed-off-by: Asahi Lina <[email protected]>
Adapted from *RFL import: kernel::io_mem Commit reference: 3dfc5eb Signed-off-by: Janne Grunau <[email protected]>
Signed-off-by: Sasha Finkelstein <[email protected]> Signed-off-by: Janne Grunau <[email protected]>
…elds aop_audio uses pin-init on `#[repr(C, packed)]` embedded into other structs. This fails with "error[E0793]: reference to packed field is unaligned" since packed struct have an alingment of 1 byte. Fixes: 42415d1 ("rust: pin-init: add references to previously initialized fields")
Used by Apple SEP driver. Signed-off-by: Sasha Finkelstein <[email protected]>
This introduces a set of bindings for working with iosys_map in rust code. The design of this is heavily based off the design for both the io and dma_map bindings for Rust. Signed-off-by: Lyude Paul <[email protected]>
…gem objects automatically"
Currently in order to implement AlwaysRefCounted for gem objects, we use a
blanket implementation:
unsafe impl<T: IntoGEMObject> AlwaysRefCounted for T { … }
While this technically works, it comes with the rather unfortunate downside
that attempting to create a similar blanket implementation in any other
kernel crate will now fail in a rather confusing way.
Using an example from the (not yet upstream) rust DRM KMS bindings, if we
were to add:
unsafe impl<T: RcModeObject> AlwaysRefCounted for T { … }
Then the moment that both blanket implementations are present in the same
kernel tree, compilation fails with the following:
error[E0119]: conflicting implementations of trait `types::AlwaysRefCounted`
--> rust/kernel/drm/kms.rs:504:1
|
504 | unsafe impl<T: RcModeObject> AlwaysRefCounted for T {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation
|
::: rust/kernel/drm/gem/mod.rs:97:1
|
97 | unsafe impl<T: IntoGEMObject> AlwaysRefCounted for T {
| ---------------------------------------------------- first implementation here
So, revert these changes for now. The proper fix for this is to introduce a
macro for copy/pasting the same implementation of AlwaysRefCounted around.
This reverts commit 38cb08c.
Signed-off-by: Lyude Paul <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
I noticed by chance that there's actually already a pointer to this in struct drm_gem_object. So, no use in carrying this around! Signed-off-by: Lyude Paul <[email protected]> Acked-by: Danilo Krummrich <[email protected]> Reviewed-by: Alice Ryhl <[email protected]> Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Janne Grunau <[email protected]>
This can be used in an unsafe way but required for the asahi driver. Signed-off-by: Janne Grunau <[email protected]>
The driver's data might not be initialized and dropping the uninitialized data will crash. Since the DRM device is expected to be released only once at reboot or poweroff leaking the device is not an issue in practice. Signed-off-by: Janne Grunau <[email protected]>
Signed-off-by: Janne Grunau <[email protected]>
With gem objects in rust, the most ideal way for us to be able to handle gem shmem object creation is to be able to handle the memory allocation of a gem object ourselves - and then have the DRM gem shmem helpers initialize the object we've allocated afterwards. So, let's spit out drm_gem_shmem_init() from drm_gem_shmem_create() to allow for doing this. Signed-off-by: Lyude Paul <[email protected]>
This is strange that in-so-far that it works on byte level and doesn't use IoSysMapRef<T>. It will be used to initialize mappings in the asahi driver either to zero or for debugging purposes to special byte patterns. Signed-off-by: Janne Grunau <[email protected]>
At the moment the way that freeing gem shmem objects is not ideal for rust bindings. drm_gem_shmem_free() releases all of the associated memory with a gem shmem object with kfree(), which means that for us to correctly release a gem shmem object in rust we have to manually drop all of the contents of our gem object structure in-place by hand before finally calling drm_gem_shmem_free() to release the shmem resources and the allocation for the gem object. Since the only reason this is an issue is because of drm_gem_shmem_free() calling kfree(), we can fix this by splitting drm_gem_shmem_free() out into itself and drm_gem_shmem_release(), where drm_gem_shmem_release() releases the various gem shmem resources without freeing the structure itself. With this, we can safely re-acquire the KBox for the gem object's memory allocation and let rust handle cleaning up all of the other struct members automatically. Signed-off-by: Lyude Paul <[email protected]>
In the future we're going to be introducing more GEM object types in rust then just gem::Object<T>. Since all types of GEM objects have refcounting, let's introduce a macro that we can use in the gem crate in order to copy this boilerplate implementation for each type: impl_aref_for_gem_obj!(). Signed-off-by: Lyude Paul <[email protected]> Reviewed-by: Daniel Almeida <[email protected]>
This is just for basic usage in the DRM shmem abstractions for implied locking, not intended as a full DMA Reservation abstraction yet. Signed-off-by: Asahi Lina <[email protected]> Signed-off-by: Daniel Almeida <[email protected]> Reviewed-by: Alice Ryhl <[email protected]> Signed-off-by: Lyude Paul <[email protected]>
For retrieving a pointer to the struct dma_resv for a given GEM object. We also introduce it in a new trait, BaseObjectPrivate, which we automatically implement for all gem objects and don't expose to users outside of the crate. Signed-off-by: Lyude Paul <[email protected]>
This is an associated type that may be used in order to specify a data-type to pass to gem objects when construction them, allowing for drivers to more easily initialize their private-data for gem objects. Signed-off-by: Lyude Paul <[email protected]> Reviewed-by: Alice Ryhl <[email protected]> Reviewed-by: Daniel Almeida <[email protected]>
The DRM shmem helper includes common code useful for drivers which allocate GEM objects as anonymous shmem. Add a Rust abstraction for this. Drivers can choose the raw GEM implementation or the shmem layer, depending on their needs. Signed-off-by: Asahi Lina <[email protected]> Signed-off-by: Daniel Almeida <[email protected]> Signed-off-by: Lyude Paul <[email protected]>
Under certain conditions (more prevalent after a suspend/resume cycle), the touchscreen controller can send the "boot complete" interrupt before it actually finished booting. In those cases, attempting to read touch data resuls in a stream of "not ready" messages being read and interpreted as a touch report. Check that the response is in fact a touch report and discard it otherwise. Reported-by: pitust <[email protected]> Closes: https://oftc.catirclogs.org/asahi/2025-12-17#34878715; Fixes: 471a92f ("Input: apple_z2 - add a driver for Apple Z2 touchscreens") Signed-off-by: Sasha Finkelstein <[email protected]>
The next display might not the same and the EDID will not necessarily be replaced on the next connect. This results in confused user space with hilarious but broken results. In kwin it resulted in a gap between two displays makling it impossible to move the mouse pointer to the other display. Signed-off-by: Janne Grunau <[email protected]>
This works around missing (or incomplete) suspend/resume handling in atc/dcp for the the HDMI output on 14 and 16-inch Macbook Pros. Signed-off-by: Janne Grunau <[email protected]>
Lockdep complains otherwise: ------------[ cut here ]------------ WARNING: CPU: 5 PID: 885 at drivers/gpu/drm/drm_gpuvm.c:1620 drm_gpuvm_bo_put+0x1b4/0x254 Modules linked in: brcmfmac_wcc uhid overlay squashfs zlib_inflate brcmfmac hci_bcm4377 brcmutil spi_nor aop_las aop_als industrialio cfg80211 fuse nfn> CPU: 5 UID: 1000 PID: 885 Comm: kwin_wayland Tainted: G S W 6.18.2+ AsahiLinux#5 PREEMPTLAZY Tainted: [S]=CPU_OUT_OF_SPEC, [W]=WARN Hardware name: Apple MacBook Pro (14-inch, M1 Pro, 2021) (DT) pstate: 61400009 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--) pc : drm_gpuvm_bo_put+0x1b4/0x254 lr : drm_gpuvm_bo_put+0x14c/0x254 sp : ffff8000893d7c10 x29: ffff8000893d7c10 x28: ffff00001b8ef9c0 x27: 0000000000000000 x26: 0000000000000002 x25: ffff800081451000 x24: dead000000000100 x23: ffff800080ee82d0 x22: ffff0000108f9d50 x21: ffff0000108f9c00 x20: ffff0000492e0700 x19: ffff000048700000 x18: 0000000000000000 x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000 x14: 0000000000000008 x13: 0000000000000000 x12: ffff8000815e34d0 x11: 0000000000000001 x10: 00000000ffffffff x9 : 0000000100000000 x8 : 0000000000000000 x7 : 0000000000000000 x6 : ffff8000807d2bcc x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000 x2 : 0000000000000000 x1 : ffff000048700258 x0 : 0000000000000000 Call trace: drm_gpuvm_bo_put+0x1b4/0x254 (P) _RINvNtCs1tcwcP3FgYC_4core3ptr13drop_in_placeINtNtNtCsgIauPoi8ikU_6kernel3drm2mm8NodeDatauNtNtCshTJcMxhWd7O_5asahi3mmu18KernelMappingInnerEEB1t_+0xb0/> _RINvNtCs1tcwcP3FgYC_4core3ptr13drop_in_placeNtNtCshTJcMxhWd7O_5asahi4file2VmEBK_+0xcc/0xf0 _RNvMNtNtCsgIauPoi8ikU_6kernel3drm4fileINtB2_4FileNtNtCshTJcMxhWd7O_5asahi4file4FileE18postclose_callbackBP_+0xac/0x2f4 drm_file_free+0x1b8/0x210 drm_release+0xb8/0x140 __fput+0xf8/0x2e4 fput_close_sync+0x44/0x114 __arm64_sys_close+0xb0/0xfc invoke_syscall+0x48/0xc8 do_el0_svc+0x7c/0xa8 el0_svc+0x3c/0xd8 el0t_64_sync_handler+0x68/0xdc el0t_64_sync+0x198/0x19c ---[ end trace 0000000000000000 ]--- Signed-off-by: Sasha Finkelstein <[email protected]>
|
manually merged to my local bits/210-gpu iirc I tried to replace the resv lock here with the gpuva lock which resulting in the object being gone at unlock time. The dma_resv_lock probably prevents this. The safety comment is non-sense but this is going to change anyway when we rebase onto the upstream gpuvm bindings. |
|
This still causea NULL pointer derefs under some conditions. Probably the same why it was missing in the first place. I hope this will be fixed by switching to Alice upstream submitted GPUVM abstractions. In the meantime I'll revert. |
This reverts commit 0475333. due to NULL ptr deref, see #433 Signed-off-by: Janne Grunau <[email protected]>
This reverts commit 0475333. due to NULL ptr deref, see #433 Signed-off-by: Janne Grunau <[email protected]>
Lockdep complains otherwise:
------------[ cut here ]------------
WARNING: CPU: 5 PID: 885 at drivers/gpu/drm/drm_gpuvm.c:1620 drm_gpuvm_bo_put+0x1b4/0x254
Modules linked in: brcmfmac_wcc uhid overlay squashfs zlib_inflate brcmfmac hci_bcm4377 brcmutil spi_nor aop_las aop_als industrialio cfg80211 fuse nfn>
CPU: 5 UID: 1000 PID: 885 Comm: kwin_wayland Tainted: G S W 6.18.2+ #5 PREEMPTLAZY
Tainted: [S]=CPU_OUT_OF_SPEC, [W]=WARN
Hardware name: Apple MacBook Pro (14-inch, M1 Pro, 2021) (DT)
pstate: 61400009 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
pc : drm_gpuvm_bo_put+0x1b4/0x254
lr : drm_gpuvm_bo_put+0x14c/0x254
sp : ffff8000893d7c10
x29: ffff8000893d7c10 x28: ffff00001b8ef9c0 x27: 0000000000000000
x26: 0000000000000002 x25: ffff800081451000 x24: dead000000000100
x23: ffff800080ee82d0 x22: ffff0000108f9d50 x21: ffff0000108f9c00
x20: ffff0000492e0700 x19: ffff000048700000 x18: 0000000000000000
x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
x14: 0000000000000008 x13: 0000000000000000 x12: ffff8000815e34d0
x11: 0000000000000001 x10: 00000000ffffffff x9 : 0000000100000000
x8 : 0000000000000000 x7 : 0000000000000000 x6 : ffff8000807d2bcc
x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
x2 : 0000000000000000 x1 : ffff000048700258 x0 : 0000000000000000
Call trace:
drm_gpuvm_bo_put+0x1b4/0x254 (P)
RINvNtCs1tcwcP3FgYC_4core3ptr13drop_in_placeINtNtNtCsgIauPoi8ikU_6kernel3drm2mm8NodeDatauNtNtCshTJcMxhWd7O_5asahi3mmu18KernelMappingInnerEEB1t+0xb0/>
RINvNtCs1tcwcP3FgYC_4core3ptr13drop_in_placeNtNtCshTJcMxhWd7O_5asahi4file2VmEBK+0xcc/0xf0
RNvMNtNtCsgIauPoi8ikU_6kernel3drm4fileINtB2_4FileNtNtCshTJcMxhWd7O_5asahi4file4FileE18postclose_callbackBP+0xac/0x2f4
drm_file_free+0x1b8/0x210
drm_release+0xb8/0x140
__fput+0xf8/0x2e4
fput_close_sync+0x44/0x114
__arm64_sys_close+0xb0/0xfc
invoke_syscall+0x48/0xc8
do_el0_svc+0x7c/0xa8
el0_svc+0x3c/0xd8
el0t_64_sync_handler+0x68/0xdc
el0t_64_sync+0x198/0x19c
---[ end trace 0000000000000000 ]---